fix(preamble): quoted tilde made Artifacts Sync and telemetry-finalize blocks dead code#2333
Open
jawadakram20 wants to merge 1 commit into
Open
fix(preamble): quoted tilde made Artifacts Sync and telemetry-finalize blocks dead code#2333jawadakram20 wants to merge 1 commit into
jawadakram20 wants to merge 1 commit into
Conversation
…e blocks dead code Bash performs tilde expansion only when `~` is UNQUOTED. Inside double quotes it is a literal character, so `"~/.claude/skills/gstack/bin/x"` is a relative path that never resolves. Two preamble generators interpolated a tilde-based `ctx.paths.binDir` inside double quotes: scripts/resolvers/preamble/generate-brain-sync-block.ts (4 sites) scripts/resolvers/preamble/generate-preamble-bash.ts (1 site) Those five sites expanded into ~250 broken lines across 49 generated SKILL.md files. Concretely: - `_BRAIN_SYNC_BIN` / `_BRAIN_CONFIG_BIN` never resolved, so the whole "Artifacts Sync (skill start)" block was inert. `artifacts_sync_mode` always read as "off" regardless of the user's real config, and the `--discover-new` / `--once` sync calls always failed (silently, behind `|| true`). - The pending-telemetry finalize guard `[ -x "~/.../gstack-telemetry-log" ]` was ALWAYS false, so the event was never logged — but the very next line still ran `rm -f "$_PF"`, deleting the pending marker. Pending telemetry was discarded unlogged rather than being flushed. Nothing errored, because every call site was guarded by `|| true` or an `if` that simply never fired. The features looked wired up and did nothing. Fix: add `quoteSafePath()` in scripts/resolvers/types.ts, which rewrites a leading `~/` to `$HOME/`, and apply it at exactly the five quoted sites. Env-var hosts (`$GSTACK_BIN`) already expand correctly when quoted and pass through untouched. Unquoted interpolations are deliberately left alone — they expand fine and keep the more readable `~` in generated docs. The 49 SKILL.md changes in this commit are regenerated output (`bun run gen:skill-docs`), not hand edits. Verified: - `[ -x "~/..." ]` → false, `[ -x "$HOME/..." ]` → true on this machine. - 0 broken quoted-tilde sites remain in generated docs (was ~250). - 770 pass / 0 fail across gen-skill-docs (incl. golden fixtures), skill-validation, skill-size-budget, brain-sync, brain-sync-windows-paths. - 64 pass / 0 fail across skill-e2e, autoplan-chain, one-way-doors, hook-scripts, question-preference-hook.
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Bash performs tilde expansion only when
~is unquoted. Inside double quotes it is a literal character, so"~/.claude/skills/gstack/bin/x"is a relative path that never resolves.Two preamble generators interpolated a tilde-based
ctx.paths.binDirinside double quotes:scripts/resolvers/preamble/generate-brain-sync-block.ts— 4 sitesscripts/resolvers/preamble/generate-preamble-bash.ts— 1 siteThose 5 source sites expanded into roughly 250 broken lines across 49 generated
SKILL.mdfiles.Impact
Artifacts Sync (skill start) was entirely inert.
_BRAIN_SYNC_BINand_BRAIN_CONFIG_BINnever resolved, soartifacts_sync_modealways read as"off"regardless of the user's actual config, and the--discover-new/--oncesync calls always failed.Pending telemetry was discarded unlogged. The finalize guard was always false:
The guard never fired, but the next line still removed the pending marker — so events were dropped rather than flushed.
Nothing errored, because every call site was guarded by
|| trueor anifthat simply never fired. The features looked wired up and did nothing.Demonstration:
Fix
Add
quoteSafePath()toscripts/resolvers/types.ts, rewriting a leading~/to$HOME/, and apply it at exactly the five quoted sites.$GSTACK_BIN) already expand correctly when quoted and pass through untouched.~.Only 3 source files change; the 49
SKILL.mdfiles in the diff are regenerated output frombun run gen:skill-docs, not hand edits.Verification